MPT-16657: add instructions and coderabbit yaml #3
Conversation
svazquezco
requested review from
albertsola and
d3rky
and removed request for
a team
📝 WalkthroughWalkthroughAdds a top-level CodeRabbit configuration file, a new Python copilot/style/testing guidance document, and removes a Danger GitHub Actions workflow. Verify config correctness, CI references, and that documentation matches repository conventions. (49 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks✅ Passed checks (3 passed)
Comment |
✅ Found Jira issue key in the title: MPT-16657 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.coderabbit.yaml.github/.instructions.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
.github/.instructions.md
32-32: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
.coderabbit.yaml (3)
45-46: Verify the base_branches configuration aligns with your branching strategy.The configuration enables auto-review for
release/*branches in addition to the default branch. Ensure this matches your intended workflow, as this will trigger automatic reviews on all release branches.
173-176: Verify that the code guidelines file pattern is correct.The configuration references
.github/.instructions.mdfor code guidelines. Confirm that this file path matches the actual location of the instructions file being added in this PR.This correctly links the CodeRabbit configuration to the coding conventions document, enabling enforcement during reviews.
181-184: LGTM! Jira integration properly configured.The Jira knowledge base integration is correctly configured with the
MPTproject key, which aligns with the title requirement on line 59 expecting theMPT-prefix.
svazquezco
force-pushed
the
MPT-16657-add-coderabitt
branch
from
e028be6 to
4066c64
Compare
svazquezco
force-pushed
the
MPT-16657-add-coderabitt
branch
from
e82a172 to
f9ec233
Compare
| - Function and variable names must be explicit and intention-revealing. | ||
| - `pyproject.toml` is the source of truth for ruff and flake8 rules. Generated code must not violate those rules. | ||
| - Follow PEP 8 unless explicitly overridden by ruff. | ||
| - Prefer simple, explicit code over clever or compact implementations. |
There was a problem hiding this comment.
Could you pls also add WPS rules here to follow + to use configuration from pyproject.toml for that
d3rky
force-pushed
the
MPT-16657-add-coderabitt
branch
from
f9ec233 to
50cca39
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.coderabbit.yaml (2)
7-64: Configuration is valid and well-structured.The review settings are properly configured. A few optional observations:
- Line 21: Sequence diagrams are disabled. Enabling them can help visualize complex code flows in the walkthrough.
- Lines 29-30: Suggested reviewers and auto-assignment are disabled. These features can improve team collaboration by automatically recommending appropriate reviewers.
- Line 59: Title requirement for "MPT-" prefix aligns well with the Jira project key configuration on line 182.
65-158: Tools configuration is valid and comprehensive.The tools section is properly configured with multiple Python linters enabled (ruff, flake8, pylint) and security scanning tools (gitleaks, semgrep) active. Past issues with
config_fileproperties have been correctly resolved.Consider verifying tool alignment with your tech stack:
- Line 112: ESLint is disabled. If the project contains JavaScript/TypeScript files, consider enabling it.
- Line 108: Checkov is disabled. If the project uses infrastructure-as-code files (Terraform, CloudFormation, etc.), consider enabling it for security checks.
📜 Review details
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.coderabbit.yaml.github/copilot-instructions.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/copilot-instructions.md
🔇 Additional comments (3)
.coderabbit.yaml (3)
1-6: LGTM! Configuration basics are well-structured.The schema reference, language setting, and feature flags are all valid and properly configured.
160-166: LGTM! Chat settings are properly configured.Auto-reply is enabled for convenience, and integration settings align with the knowledge base configuration (Jira auto-enabled, Linear disabled).
167-187: Excellent knowledge base configuration!The knowledge base settings are well-configured with good integration between components:
- Line 174: Code guidelines reference
.github/copilot-instructions.md, which aligns with the other file being added in this PR.- Line 182: Jira project key "MPT" matches the title requirement prefix on line 59.
- Web search and appropriate scope settings ensure comprehensive context for reviews.
svazquezco
force-pushed
the
MPT-16657-add-coderabitt
branch
from
50cca39 to
d2e4b88
Compare
svazquezco
force-pushed
the
MPT-16657-add-coderabitt
branch
5 times, most recently
from
6cebdf4 to
a82326e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
69-107: Custom pre-merge checks are well-designed.The three custom checks are properly configured and meet all schema requirements:
- Clear, deterministic pass/fail criteria
- Appropriate severity levels (error for Jira key requirement, warnings for best practices)
- Comprehensive instructions with specific output formats
The checks effectively enforce your team's workflow requirements around Jira integration, test coverage, and commit hygiene.
💡 Optional: Consider formatting improvement for readability
Line 102 contains a very long instruction string. While valid, consider using YAML's block scalar style for better readability:
- - Post: ⚠️ This PR contains X commits. Please squash them into a single commit to keep the git history clean and easy to follow. Multiple commits are acceptable only in the following cases: One commit is a technical refactoring, and another introduces business logic changes. You are doing a complex multi-step refactoring (although in this case we still recommend splitting it into separate PRs). + - Post: >- + ⚠️ This PR contains X commits. Please squash them into a single commit to keep the git history clean and easy to follow. + Multiple commits are acceptable only in the following cases: + One commit is a technical refactoring, and another introduces business logic changes. + You are doing a complex multi-step refactoring (although in this case we still recommend splitting it into separate PRs).
📜 Review details
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.coderabbit.yaml.github/copilot-instructions.md
✅ Files skipped from review due to trivial changes (1)
- .github/copilot-instructions.md
🔇 Additional comments (4)
.coderabbit.yaml (4)
116-117: ✅ Previous issues resolved!Great work addressing the previous review feedback! Both the
ruff(lines 116-117) andflake8(lines 156-157) configurations have been corrected by removing the unsupportedconfig_fileproperties. These tools will now rely on their automatic configuration file discovery, which is the recommended approach.Also applies to: 156-157
1-7: LGTM - Basic configuration is well-structured.The basic configuration is valid and properly formatted. The tone instructions align well with the goal of tracking resolved issues across commits.
12-17: Excellent Jira integration in summary instructions.The high-level summary instructions are well-crafted to automatically extract and link Jira issues in the PR description. This will ensure consistent formatting and proper issue tracking across your PRs.
210-229: Knowledge base configuration is properly set up.The knowledge base settings are well-configured:
- Code guidelines enabled with default file patterns (which includes
.github/copilot-instructions.mdbeing added in this PR)- Jira integration properly enabled with the
MPTproject key, consistent with your custom checks- Appropriate use of
autoscope for flexibility between public and private repositories
svazquezco
force-pushed
the
MPT-16657-add-coderabitt
branch
from
a82326e to
4f8ded0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
2-2: Consider using a more specific language code.The language is set to
"en"which is valid, but the schema default is"en-US". Consider whether a specific locale like"en-US","en-GB", or another variant better matches your team's preference.
📜 Review details
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.coderabbit.yaml.github/copilot-instructions.md.github/workflows/danger.yml
💤 Files with no reviewable changes (1)
- .github/workflows/danger.yml
✅ Files skipped from review due to trivial changes (1)
- .github/copilot-instructions.md
🔇 Additional comments (5)
.coderabbit.yaml (5)
40-41: Verify the exclusion of.gitignoreis intentional.The path_filters configuration excludes
.gitignorefrom reviews. This is unusual since.gitignoreis typically either auto-excluded or would be included in reviews for consistency checks. Please confirm this exclusion is intentional for your workflow.
72-110: Well-structured custom pre-merge checks.The three custom checks are clearly defined with appropriate modes and detailed instructions:
- Jira issue key validation (error mode)
- Test coverage reminder (warning mode)
- Single commit enforcement (warning mode)
The instructions provide clear pass/fail criteria and user-friendly messaging.
111-204: ✅ Previous configuration issues resolved; tools configuration looks good.The invalid
config_fileproperties forruff(previously at lines 73-75) andflake8(previously at lines 114-116) have been successfully removed in commit 4066c64. Both tools now correctly use only theenabledproperty and will auto-discover their configuration files.The tools configuration is well-structured:
- Python linting suite enabled: ruff, flake8, pylint
- Security scanning enabled: gitleaks, semgrep, osvScanner
- Infrastructure tools enabled: yamllint, hadolint, actionlint, checkov disabled but others active
- Language-specific tools appropriately disabled for non-applicable languages
205-212: Chat configuration is appropriate.The chat settings are configured correctly:
- Art disabled for cleaner output
- Auto-reply enabled for better UX
- Jira integration set to "auto" (enabled for private repos)
- Linear integration disabled
213-232: Knowledge base configuration is well-aligned with project needs.The knowledge base settings show good consistency with the project:
- Jira integration explicitly enabled with
MPTproject key (line 227)- Matches the custom pre-merge check for MPT-XXXX issue keys (line 75)
- Code guidelines enabled to use defaults including
.github/copilot-instructions.md(line 219)- Web search enabled for enhanced context
- Appropriate scope settings using "auto" for privacy-aware defaults
All settings are valid per schema.
svazquezco
force-pushed
the
MPT-16657-add-coderabitt
branch
from
4f8ded0 to
8aac4e7
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.coderabbit.yaml (3)
42-44: Consider clarifying the formatting instruction.The instruction "Do not suggest formatting changes or mention code formatting using Black. Instead use Ruff or Flake8" could be clearer since Ruff can format (like Black) while Flake8 only lints. Consider rewording to distinguish formatting from linting concerns.
🔎 Suggested clarification
- path: "**/*.py" instructions: | - Do not suggest formatting changes or mention code formatting using Black. Instead use Ruff or Flake8 + For formatting suggestions, prefer Ruff over Black. For linting concerns, use Ruff or Flake8.
87-99: Clarify what constitutes "code files" in the test coverage check.The instruction checks if "code files" are modified but doesn't specify what qualifies as a code file (e.g.,
*.py,*.js,*.java). This ambiguity could lead to inconsistent results. Consider specifying file patterns or extensions that should trigger the test coverage requirement.🔎 Suggested improvement
- name: "Test Coverage Required" mode: "warning" instructions: | - Check if the PR modifies any code files but does not include changes in the tests/ folder. + Check if the PR modifies any code files (*.py, *.js, *.ts, etc.) but does not include changes in the tests/ folder. If code files are modified but no test files are changed: - Count the number of code files modified - Post: This PR modifies code (X file(s)) but does not include any changes in the tests/ folder. Please consider adding or updating tests to cover your changes. - Replace X with the actual number of files - Mark the check as FAILED If test files are included or no code files are modified: - Mark the check as PASSED
121-122: Consider the overlap between Python linting tools.You have enabled three Python linters: Ruff (line 122), Flake8 (line 162), and Pylint (line 190). Ruff is a modern linter that already includes many Flake8 rules and is significantly faster. Running all three tools may produce redundant or conflicting feedback.
Consider keeping only Ruff enabled, or clearly document why multiple Python linters are necessary for your workflow.
Also applies to: 161-162, 189-190
📜 Review details
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.coderabbit.yaml.github/copilot-instructions.md.github/workflows/danger.yml
💤 Files with no reviewable changes (1)
- .github/workflows/danger.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/copilot-instructions.md
🔇 Additional comments (1)
.coderabbit.yaml (1)
211-212: Verify Jira integration usage consistency.The Jira integration has different settings in two places:
chat.integrations.jira.usage: auto(line 212) - enables only for private repositoriesknowledge_base.jira.usage: enabled(line 227) - always enabled, even for public repositoriesThis difference may be intentional (e.g., you want Jira knowledge base for all repos but chat integration only for private ones), but please verify this is the desired behavior.
Also applies to: 226-227
Closes MPT-16657 MPT-16657